fix(antigravity): use global config directory for mcp_config.json#918
fix(antigravity): use global config directory for mcp_config.json#918moferrin wants to merge 20 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAntigravity MCP config writes now target the shared global Gemini config path. Variant directory resolution now prefers OS-specific IDE locations before desktop and CLI fallbacks. Engram injection now uses the default MCP tool args and updated tests reflect the new paths. ChangesAntigravity MCP Config and Variant Resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates Antigravity MCP config handling to use a single shared global config file location, and aligns tests/golden assertions with the new path.
Changes:
- Switch Antigravity
MCPConfigPathfrom variant-specific directories to~/.gemini/config/mcp_config.json. - Update golden test and injection tests to read/verify MCP config in the new global location.
- Adjust adapter path tests to expect the shared MCP config path for both CLI-only and Desktop-only setups.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/components/golden_test.go | Updates golden test to read MCP config from the new shared global config directory. |
| internal/components/engram/inject_test.go | Renames test and updates expected MCP config output path to the shared global location. |
| internal/agents/antigravity/adapter_test.go | Updates adapter path tests to expect the global MCP config path. |
| internal/agents/antigravity/adapter.go | Changes MCPConfigPath implementation to always point at the shared global config file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cliMCPPath := filepath.Join(home, ".gemini", "config", "mcp_config.json") | ||
| content, err := os.ReadFile(cliMCPPath) | ||
| if err != nil { | ||
| t.Fatalf("ReadFile(%q) error = %v", cliMCPPath, err) |
| // MCP config written to ~/.gemini/config/mcp_config.json. | ||
| mcpJSON := readTestFile(t, filepath.Join(home, ".gemini", "config", "mcp_config.json")) |
| // Antigravity uses a shared global config for MCP across CLI, Desktop, and IDE. | ||
| return filepath.Join(homeDir, ".gemini", "config", "mcp_config.json") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/components/engram/inject_test.go`:
- Line 571: The variable name cliMCPPath is misleading since the MCP config is
now stored in a global/shared location rather than a CLI-specific directory.
Rename the cliMCPPath variable to globalMCPPath (or mcpConfigPath) to accurately
reflect its purpose, and update all references to this variable throughout the
test file to maintain consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b39ab65d-ac1d-4de8-ae2c-2ed325d12f61
📒 Files selected for processing (4)
internal/agents/antigravity/adapter.gointernal/agents/antigravity/adapter_test.gointernal/components/engram/inject_test.gointernal/components/golden_test.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
| // GlobalMCPConfigDir is the directory path relative to $HOME where Antigravity | ||
| // stores its shared MCP configuration. Exported so tests can build paths | ||
| // without duplicating magic strings. | ||
| var GlobalMCPConfigDir = filepath.Join(".gemini", "config") | ||
|
|
||
| // GlobalMCPConfigFile is the filename used for the shared MCP configuration. | ||
| const GlobalMCPConfigFile = "mcp_config.json" |
| func (a *Adapter) MCPConfigPath(homeDir string, _ string) string { | ||
| return filepath.Join(a.antigravityVariantDir(homeDir), "mcp_config.json") | ||
| // Antigravity uses a shared global config for MCP across CLI, Desktop, and IDE. | ||
| return filepath.Join(homeDir, GlobalMCPConfigDir, GlobalMCPConfigFile) | ||
| } |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/components/engram/inject_test.go (1)
571-581: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that the retired CLI MCP path is not written.
This proves the new global file is written, but it would still pass if
Injectalso kept writing~/.gemini/antigravity-cli/mcp_config.json, which is the path this PR is meant to retire. The negative guard later in the test targets a different legacy location.🧪 Possible assertion
globalMCPPath := antigravity.GlobalMCPConfigPath(home) content, err := os.ReadFile(globalMCPPath) if err != nil { t.Fatalf("ReadFile(%q) error = %v", globalMCPPath, err) } + oldCLIPath := filepath.Join(home, ".gemini", "antigravity-cli", "mcp_config.json") + if _, err := os.Stat(oldCLIPath); !os.IsNotExist(err) { + t.Fatalf("legacy CLI MCP path %q should not be written for antigravity; stat err = %v", oldCLIPath, err) + } text := string(content)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/components/engram/inject_test.go` around lines 571 - 581, The Inject test currently verifies the new Antigravity global MCP config contents but does not assert that the retired CLI MCP config path is no longer written. Update the test around globalMCPPath in inject_test.go to also check the legacy Antigravity CLI MCP location is absent after Inject runs, using the existing Inject flow and the retired path symbol(s) already used elsewhere in the test to make sure the migration fully stops writing the old file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agents/antigravity/adapter_test.go`:
- Around line 37-49: The Windows fallback path in the Antigravity IDE setup is
currently only covered when APPDATA is empty, so the new home/AppData/Roaming
branch is not exercised on Windows builders where APPDATA is normally set.
Update the test around the runtime.GOOS switch in adapter_test.go to add a
Windows-only case that explicitly verifies the fallback behavior when APPDATA is
unset, using the existing ideDir construction logic and the Antigravity IDE path
handling to keep the branch covered.
---
Outside diff comments:
In `@internal/components/engram/inject_test.go`:
- Around line 571-581: The Inject test currently verifies the new Antigravity
global MCP config contents but does not assert that the retired CLI MCP config
path is no longer written. Update the test around globalMCPPath in
inject_test.go to also check the legacy Antigravity CLI MCP location is absent
after Inject runs, using the existing Inject flow and the retired path symbol(s)
already used elsewhere in the test to make sure the migration fully stops
writing the old file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: df1d2fa6-aa08-44ed-b2fa-8e4cc1db6925
📒 Files selected for processing (4)
internal/agents/antigravity/adapter.gointernal/agents/antigravity/adapter_test.gointernal/components/engram/inject.gointernal/components/engram/inject_test.go
💤 Files with no reviewable changes (1)
- internal/components/engram/inject.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
config_scan.go hardcoded the Antigravity path to ~/.gemini/antigravity-cli, missing IDE and Desktop variants. Add antigravityConfigDir() helper that mirrors the adapter's variant resolution (IDE → Desktop → CLI fallback) without importing the agents package (import cycle). Marked as a TODO shim until the cycle is broken.
Antigravity variants install into ~/.gemini/antigravity-ide, ~/.gemini/antigravity, and ~/.gemini/antigravity-cli natively. This commit removes OS-specific APPDATA legacy checks that caused permission/fallback issues and prioritizes variants sequentially. Additionally, it appends a '(shared global configuration)' suffix to mcp_config.json writes in the CLI report to increase visibility, and fixes an unrelated .exe flake in the beta engram installation test.
69a6229 to
ed58d05
Compare
ed58d05 to
a12a74b
Compare
a12a74b to
016391c
Compare
…den tests Also updates golden files that had mismatched expectations.
|
First of all, my bad on the To be 100% sure this time, I installed all three Antigravity variants on my PC and validated their actual folders. Doing this, we also realized that the old fallback in
With this PR, we structurally fix the root issue: Does this priority order seem correct to you? If you think the order should be different, or if you believe it would be better to have a way to configure each variant independently, let me know and we can iterate on it. I think this structure covers all edge cases now, but let me know what you think. |
🔗 Linked Issue
Closes #779
🏷️ PR Type
type:bug— Bug fix (non-breaking change that fixes an issue)type:feature— New feature (non-breaking change that adds functionality)type:docs— Documentation onlytype:refactor— Code refactoring (no functional changes)type:chore— Build, CI, or tooling changestype:breaking-change— Breaking change📝 Summary
Updates the Antigravity
MCPConfigPathto point to the global config directory (~/.gemini/config/mcp_config.json) instead of the variant-specific path (~/.gemini/antigravity-cli/mcp_config.json).Problem Context:
Antigravity enforces a configuration hierarchy where the global configuration file (
~/.gemini/config/mcp_config.json) takes absolute priority. This global config file is automatically created by Antigravity when it is installed.Because this global file exists (even if it is empty), Antigravity reads it, assumes its state, and completely ignores the CLI-specific file (
~/.gemini/antigravity-cli/mcp_config.json) that Gentle AI just finished configuring.As a result, Engram MCP tools are never injected into the session (tools like
mem_save,mem_context, etc., do not appear as callable actions in the agent's toolset), forcing users to investigate why the MCP is unavailable even though the Engram CLI works perfectly.Official Documentation Alignment:
This change perfectly aligns with the Antigravity MCP documentation which now explicitly specifies that the configuration file is located at
~/.gemini/config/mcp_config.json.By making this change and writing the configuration directly to the shared global directory, we ensure that:
📂 Changes
internal/agents/antigravity/adapter.goMCPConfigPathand OS-specific variant checksinternal/agents/antigravity/adapter_test.gointernal/components/engram/inject.go--tools=agentexceptioninternal/components/engram/inject_test.gointernal/components/golden_test.go🧪 Test Plan
Unit Tests
go test ./...E2E Tests (Docker required)
go test ./...)cd e2e && ./docker-test.sh)✅ Contributor Checklist
status:approvedsize:exceptionwith rationale documentedtype:*label to this PRgo test ./...)cd e2e && ./docker-test.sh)Co-Authored-BytrailersSummary by CodeRabbit
New Features
Bug Fixes
Documentation